Navigation: Reduce include dependencies with forward declares#117557
Navigation: Reduce include dependencies with forward declares#117557akien-mga wants to merge 1 commit intogodotengine:masterfrom
Conversation
| #include "scene/resources/shader_include.h" | ||
|
|
||
| class ShaderInclude; |
There was a problem hiding this comment.
That's the only non-navigation change in the PR. I initially set out to minimize includes of scene headers in servers headers, which is how I fell into this navigation rabbit hole.
akien-mga
force-pushed
the
navigation-includes-cleanup
branch
2 times, most recently
from
f3d4408 to
af3d3ea
Compare
| class NavAgent2D; | ||
| class NavLink2D; | ||
| class NavMap2D; | ||
| class NavObstacle2D; | ||
| class NavRegion2D; |
There was a problem hiding this comment.
Seems like MSVC doesn't like the use of forward declared types in RID_Owner, unlike GCC and Clang:
Error: .\core/templates/rid_owner.h(431): error C2027: use of undefined type 'NavObstacle2D'
D:\a\godot\godot\modules\navigation_2d\2d/godot_navigation_server_2d.h(42): note: see declaration of 'NavObstacle2D'
.\core/templates/rid_owner.h(431): note: the template instantiation context (the oldest one first) is
D:\a\godot\godot\modules\navigation_2d\2d/godot_navigation_server_2d.h(81): note: see reference to class template instantiation 'RID_Owner<NavObstacle2D,false>' being compiled
.\core/templates/rid_owner.h(518): note: see reference to class template instantiation 'RID_Alloc<T,false>' being compiled
with
[
T=NavObstacle2D
]
.\core/templates/rid_owner.h(424): note: while compiling class template member function 'RID_Alloc<T,false>::~RID_Alloc(void)'
with
[
T=NavObstacle2D
]
Error: .\core/templates/rid_owner.h(94): error C2079: 'RID_Alloc<T,false>::Chunk::data' uses undefined class 'NavObstacle2D'
with
[
T=NavObstacle2D
]
.\core/templates/rid_owner.h(94): note: the template instantiation context (the oldest one first) is
.\core/templates/rid_owner.h(93): note: while compiling class 'RID_Alloc<T,false>::Chunk'
with
[
T=NavObstacle2D
]
Error: .\core/templates/rid_owner.h(439): error C2325: 'T': unexpected type to the right of '.~': expected '<error type>'
with
[
T=NavObstacle2D
]
There was a problem hiding this comment.
I reverted those forward declares since MSVC doesn't like them.
godot_navigation_server_*.h is only included in register_types.cpp anyway so forward declares here weren't particularly beneficial.
akien-mga
force-pushed
the
navigation-includes-cleanup
branch
from
af3d3ea to
496ee51
Compare
StarryWorm
left a comment
StarryWorm
left a comment
There was a problem hiding this comment.
Total includes project-wide go from 187973 to 187461 with this PR1, so -512 or -0.27%. That's actually a kind of huge improvement for such a small change.
LGTM!
1 Data obtained by cherry-picking the commit to a new branch off of master as the PR branch is a bit behind. Numbers only reflect total includes by *.cpp files.
akien-mga
force-pushed
the
navigation-includes-cleanup
branch
from
496ee51 to
7b19202
Compare
akien-mga
force-pushed
the
navigation-includes-cleanup
branch
from
7b19202 to
a165500
Compare
| #include "core/object/class_db.h" | ||
| #include "scene/3d/skeleton_3d.h" | ||
| #include "scene/main/scene_tree.h" | ||
| #include "servers/rendering/rendering_server.h" |
There was a problem hiding this comment.
This may have been missing in the master branch too, I found it by compiling this PR with target=template_release disable_navigation_2d=yes disable_navigation_3d=yes.
4 participants
I checked manually all
navigation_*andnav_*headers to see what includes could be removed or replaced by forward declares, to reduce incremental recompilation time.